[codex] OM-SEC-19: Protect migration and SSH setup authorization - #9474
[codex] OM-SEC-19: Protect migration and SSH setup authorization#9474AFOliveira wants to merge 55 commits into
Conversation
Share the existing security helpers, propagate command-scoped sudo through the direct package helper boundary, and keep the regression focused on the reusable-credential defect.
f66c372 to
94a84d7
Compare
|
Reworked after the shared-helper review: force-pushed 94a84d7. The PR now excludes the rejected trusted-source-root proposal from closed #9351 and is limited to the real migration sudo-lifetime boundary. The shared libraries, package helper, and no-update wrapper are byte-identical prerequisite copies from #9457/#9470/#9469. Local verification was rerun on the audit baseline and after a clean cherry-pick onto current quattro b71dcad: exploit baseline succeeds/candidate blocks, focused 11/11, mutation detected, CLI 116/116, shell 224/225 with only the base-identical launch-about animation failure. |
There was a problem hiding this comment.
🟡 Changes recommended
Failure and signal paths bypass final sudo revocation, while the direct package-helper boundary remains untested.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Hardens migrations against reusable sudo authorization across mixed-trust execution.
Changes:
- Adds command-scoped sudo and shared security helpers.
- Sanitizes migration startup and sudo handling.
- Adds isolated migration regressions.
File summaries
| File | Description |
|---|---|
bin/omarchy-migrate |
Enforces hardened migration execution. |
bin/omarchy-pkg-add |
Supports no-update sudo mode. |
bin/omarchy-security-functions |
Adds shared security primitives. |
bin/omarchy-install-security-functions |
Adds startup sanitization helpers. |
default/omarchy/sudo-no-update/sudo |
Wraps sudo with -N. |
test/shell.d/migrate-scope-test.sh |
Tests migration scope and ordering. |
test/shell.d/migrate-wrapper-test.sh |
Tests the isolated wrapper environment. |
Review details
- Files reviewed: 3/7 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
One issue I found: The second revocation runs only after every migration succeeds. With set -e, a failed or interrupted migration exits before line 158. If migration code bypasses the PATH wrapper, for example through /usr/bin/sudo or another PATH-resetting helper, it can leave a reusable timestamp behind after the aborted queue. This preserves part of credential-leak scenario the PR is intended to close. |
…view-9469-20260906
AFOliveira
changed the title
…dex/portfolio-9457-20260907
…rtfolio-9463-20260907 # Conflicts: # manual/48-security.md
…rtfolio-9474-20260907
…rtfolio-9474-20260907 # Conflicts: # bin/omarchy-setup-security-sshd # test/shell.d/setup-security-sshd-test.sh
There was a problem hiding this comment.
🟡 Changes recommended
T2 completion can be published without the required successful persistent rebuild, and some stated security coverage remains disabled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 39/54 changed files
- Comments generated: 4
- Review effort level: Balanced
| if [[ -f $limine_conf ]] && /usr/bin/grep -q 'pm_async=off' "$limine_conf" && | ||
| /usr/bin/grep -q 'mem_sleep_default=deep' "$limine_conf"; then rebuild=1; fi | ||
| if (( rebuild )); then /usr/bin/limine-mkinitcpio || return 1; fi | ||
| /usr/bin/install -Dm644 /dev/null "$repair_marker" || return 1 |
| if false && (( ${#namespace_args[@]} > 0 )) && | ||
| "${namespace_args[@]}" /usr/bin/bash -c ' |
| ├── post-update.d/ # At the end of `omarchy update`, after privileged work | ||
| ├── pre-refresh-pacman.d/ # After `omarchy refresh pacman` finishes (legacy name) |
| Channel switching defers the legacy `pre-refresh-pacman` hook across the package | ||
| swap and the complete update. The hook runs exactly once at the final cold | ||
| credential boundary; it is skipped if the composite operation fails earlier. |
Keep the ordered migration queue and SSH setup outside reusable sudo lifetime. Startup and source-root validation happen before selecting code; authentication is revoked before work, after each successful migration, and on failure or catchable signals. Completion is published only after both work and revocation succeed. Direct package-helper calls preserve the same boundary.
Finding: OM-SEC-19. Sean Huber reported inherited Bash-state authorization reuse in migrations and SSH setup. Afonso Oliveira independently disclosed the migration lifetime and direct package-helper paths as OM-SEC-19 and authored this remediation.
Order: #9457 → #9469 → #9467, then this PR together with #9463. This branch includes the exact corrected SSH implementation from #9463
1fc0511c, preserving machine-wide key/admission proof, native policy checks, publication ordering and rollback. #9473 remains closed and is not a prerequisite. Reconcile the separate AUR/migration work in #9795 and override handling in #10022 when integrating them.Four audited machine repairs now use one fixed installed transaction each: T2 defaults, Bluetooth power persistence, FIDO2 authfile ownership and CUPS hardening. Each has a literal target, closed machine arguments, a clean environment, root-side serialization/recheck and no nested sudo. Discovery errors and failed mutations remain retryable. The FIDO2 repair handles a credential hidden behind a root-only directory; T2 completion requires a successful persistent rebuild even when running kernel arguments already look correct.
Final revision:
17a30769a049e3961bfffb1a852a7d4859ff49c4. Independent Daybreak reassessment found no remaining code or focused-fixture blocker. Focused migration, machine-body, retry, serialization, SSH, startup and package-helper checks plus 112 CLI checks pass. Transformed complete production scripts execute against harmless fixed stand-ins in an exactly probed user/mount namespace; argument/root gates check the actual diagnostic, and retry tests cover partial progress. The companion SSH final-policy migration also passed installed two-account VM validation with one password prompt. A native coldsudo -Nfixture command confirms it can authenticate without updating the cache; stub invocations are not claimed as visible password counts.Still draft for usability and installed-system validation. The four fixed phases improve their own authorization structure, but the historical 58-prompt migration queue remains unresolved and has not been remeasured on a complete legacy upgrade. The earlier 163-prompt Quattro run is an unacceptable UX result, not readiness evidence. Real T2/initramfs, FIDO2, Bluetooth, printer and full-queue cancellation/retry validation remain required. This branch must not restore broad reusable authorization to meet the prompt budget.